account/accountdomain/user/auth_test.go (1)

33-33: Add test cases for edge cases.

While the current test case covers the happy path, consider adding test cases for:

• Empty string input
• Invalid UUID format
• Special characters in the input

 func TestGenReearthSub(t *testing.T) {
 	uid := accountdomain.NewUserID()
 
 	tests := []struct {
 		name  string
 		input string
 		want  *Auth
 	}{
 		{
 			name:  "should return reearth sub",
 			input: uid.String(),
 			want: &Auth{
 				Provider: "reearth",
 				Sub:      "reearth|" + uid.String(),
 			},
 		},
+		{
+			name:  "empty string",
+			input: "",
+			want: &Auth{
+				Provider: "reearth",
+				Sub:      "reearth|",
+			},
+		},
+		{
+			name:  "invalid uuid",
+			input: "invalid-uuid",
+			want: &Auth{
+				Provider: "reearth",
+				Sub:      "reearth|invalid-uuid",
+			},
+		},
 	}

Also applies to: 190-212

mongox/pagination_test.go (1)

267-268: Maintain consistent function parameter formatting

The line breaks in the function parameters are inconsistent. Consider maintaining a single style:

-	}, i int,
-	) any {
+	}, i int) any {

Also applies to: 341-342, 442-443

util/list_test.go (2)

74-80: Test implementation looks good, consider adding more edge cases.

The test correctly verifies the core functionality of Copy(). Consider enhancing it with additional test cases:

• Empty list behavior
• Lists with different sizes
• Lists with nil elements in between

---

Line range hint 956-1124: Consider improving code formatting for better readability.

The indentation in the test data structure is inconsistent. Consider standardizing the indentation, particularly in the member initialization blocks.

 Members(map[user.ID]workspace.Member{
-			userID:  {Role: workspace.RoleOwner},
-			userID2: {Role: workspace.RoleReader},
+     userID:  {Role: workspace.RoleOwner},
+     userID2: {Role: workspace.RoleReader},
 }).Personal(true).MustBuild()

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

#!/bin/bash
# Check for breaking changes in mongo-driver
gh api graphql -f query='
{
  repository(owner: "mongodb", name: "mongo-go-driver") {
    releases(first: 10, orderBy: {field: CREATED_AT, direction: DESC}) {
      nodes {
        tagName
        description
      }
    }
  }
}
'

account/accountdomain/user/theme.go (1)

Add test coverage for Theme.Valid() method

The Valid() method is currently not covered by any tests in theme_test.go. Please add test cases to verify:

• Valid themes (ThemeDark, ThemeLight, ThemeDefault) return true
• Invalid theme values return false

Example test structure:

func TestTheme_Valid(t *testing.T) {
    assert.True(t, ThemeDark.Valid())
    assert.True(t, ThemeLight.Valid())
    assert.True(t, ThemeDefault.Valid())
    assert.False(t, Theme("invalid").Valid())
}

🔗 Analysis chain

Line range hint 27-33: Verify test coverage for the new Valid method.

Since this PR focuses on test improvements, let's ensure adequate test coverage for this new functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing tests of the Theme.Valid() method

# Look for test files and test cases related to Theme validation
rg -l "TestTheme.*Valid|Test.*Theme.*Valid" --type go

# Look for actual test implementations
ast-grep --pattern 'func Test$_($_) {
  $$$
  Valid()
  $$$
}'

Length of output: 120

---

Script:

#!/bin/bash
# Let's try a broader search for test files related to Theme and then examine their contents

# First, find test files that might contain Theme-related tests
rg -l "Theme" --type go "*_test.go"

# Then, let's look for any Theme-related test implementations
rg "func Test.*Theme" -A 5 --type go

# Also search for any usage of Theme in test files
rg "Theme" -B 2 -A 2 --type go "*_test.go"

Length of output: 1067

---

Script:

#!/bin/bash
# Let's examine the theme_test.go file content to check for Valid() method tests
cat account/accountdomain/user/theme_test.go

# Also let's check the implementation file to understand the full context
cat account/accountdomain/user/theme.go

Length of output: 1082

account/accountusecase/accountinteractor/user.go (1)

Line range hint 294-334: Consider adding rate limiting for password reset requests

To prevent abuse, consider implementing rate limiting for password reset requests per email address.

Improve error handling for non-existent email

Currently, when an email is not found, it returns the internal error. For security reasons, consider returning a generic message to avoid email enumeration attacks.

 u, err := i.repos.User.FindByEmail(ctx, email)
 if err != nil {
-    return err
+    if errors.Is(err, rerror.ErrNotFound) {
+        // Return success even when email not found to prevent email enumeration
+        return nil
+    }
+    return err
 }

Consider extracting email preparation logic

The email preparation logic could be extracted into a separate method to improve readability and maintainability.

+ func (i *User) preparePasswordResetEmail(u *user.User, pr *user.PasswordReset) (string, string, error) {
+     var TextOut, HTMLOut bytes.Buffer
+     link := i.authSrvUIDomain + "/?pwd-reset-token=" + pr.Token
+     content := passwordResetMailContent
+     content.UserName = u.Name()
+     content.ActionURL = htmlTmpl.URL(link)
+
+     if err := authTextTMPL.Execute(&TextOut, content); err != nil {
+         return "", "", err
+     }
+     if err := authHTMLTMPL.Execute(&HTMLOut, content); err != nil {
+         return "", "", err
+     }
+     return TextOut.String(), HTMLOut.String(), nil
+ }

authserver/storage.go (1)

Based on the search results, I can now provide a final response since I have found the implementation details of both the Request struct and the MustBuild() method in the codebase:

Consider using Build() instead of MustBuild() to handle validation errors gracefully

The MustBuild() method in request_builder.go will panic if the request ID is nil, as shown in its implementation. While this is currently the only validation, panics should be avoided in production code. Use the regular Build() method instead, which returns an error that can be properly handled:

request, err := NewRequest().
    NewID().
    ClientID(authReq.ClientID).
    State(authReq.State).
    ResponseType(authReq.ResponseType).
    Scopes(authReq.Scopes).
    Audiences(audiences).
    RedirectURI(authReq.RedirectURI).
    Nonce(authReq.Nonce).
    CodeChallenge(cc).
    AuthorizedAt(nil).
    Build()
if err != nil {
    return nil, err
}

🔗 Analysis chain

Line range hint 153-164: Handle potential panics from MustBuild()

The MustBuild() method suggests it may panic if the request object is in an invalid state. Consider using the regular Build() method (if available) and handle any validation errors gracefully.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Build() method implementation to verify panic behavior
ast-grep --pattern 'func $_($$_) {
  $$$
  panic($$$)
  $$$
}'

Length of output: 64

---

Script:

#!/bin/bash
# Search for MustBuild and Build methods
rg "MustBuild|Build" -A 5 -B 5

Length of output: 82091

---

Script:

#!/bin/bash
# Search for Request struct and its methods
ast-grep --pattern 'type Request struct {
  $$$
}'

Length of output: 725

---

Script:

#!/bin/bash
# Search for NewRequest function
ast-grep --pattern 'func NewRequest() $_'

Length of output: 219

account/accountdomain/workspace/id.go (1)

13-17: LGTM! Consider adding documentation for the grouped declarations.

The variable grouping is clean and consistent with the type declarations above. While the code is well-organized, it might be helpful to add a brief comment above each group explaining their purpose, especially since these are exported identifiers.

Consider adding documentation like this:

+// ID types for workspace-related entities
 type (
   ID            = accountdomain.WorkspaceID
   // ...
 )

+// Constructor functions for workspace-related IDs
 var (
   NewID            = accountdomain.NewWorkspaceID
   // ...
 )

account/accountdomain/user/theme.go (2)

27-27: Consider adding GoDoc comment for the Valid method.

Adding documentation would help users understand the purpose and behavior of this validation method.

+// Valid returns true if the Theme is one of the predefined valid themes (ThemeDark, ThemeLight, or ThemeDefault).
 func (t Theme) Valid() bool {

---

Line range hint 27-33: Consider integrating validation with ThemeFrom.

The ThemeFrom function could benefit from using this validation to ensure it only returns valid themes.

 func ThemeFrom(s string) Theme {
-	switch strings.ToLower(s) {
+	t := Theme(strings.ToLower(s))
+	switch t {
 	case "dark":
 		return ThemeDark
 	case "light":
 		return ThemeLight
 	}
 	return ThemeDefault
 }

account/accountusecase/accountinteractor/user.go (1)

292-292: Consider using structured error handling

The error message for expired verification could be more consistent with the codebase's error handling patterns.

Consider using the i18n package for the error message:

- return nil, errors.New("verification expired")
+ return nil, rerror.NewE(i18n.T("verification expired"))

authserver/storage.go (1)

Line range hint 153-164: LGTM! Consider adding input validation.

The refactoring to use method chaining improves code readability. However, consider adding validation for the input parameters before using them in the builder chain.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

#!/bin/bash
# Description: Check for similar type/var grouping patterns in related files

# Look for similar type and var grouping patterns in related files
echo "Checking type declaration patterns:"
rg -A 5 "^type \($" "account/"

echo -e "\nChecking var declaration patterns:"
rg -A 5 "^var \($" "account/"

 passwordReset := u.PasswordReset()
-ok := passwordReset.Validate(token)
+ok := passwordReset.Validate(token) && !passwordReset.IsExpired()
 if !ok {
     return accountinterfaces.ErrUserInvalidPasswordReset
 }